-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(2/2) Add support for all reportAction types in ChatListItem - use ReportActionItem in ChatListItem #54228
base: main
Are you sure you want to change the base?
(2/2) Add support for all reportAction types in ChatListItem - use ReportActionItem in ChatListItem #54228
Conversation
…x/51296-chat-list-item
…x/51296-chat-list-item
@luacmartins @allgandalf this is draft PR for the second part. Will the logic to show icons the same between reportactionitem and chatlistitem? PureReportActionItem might render ReportActionItemSingle and use getReportActionActorAccountID, to determine the icons from personalDetails, but SearchReportAction doesn't have ownerAccountID, actorAccountID, etc to fullfill the logic. will |
@wildan-m Search already returns |
PR to add |
…x/51296-chat-list-item
@luacmartins @allgandalf There is a new ESLint rule that enforces the use of a default value of '0' or undefined instead of '-1'. I am concerned that this change could potentially cause regressions, especially because there are numerous occurrences in the PureReportActionItem, ReportActionItemSingle and their subcomponents. Should this be addressed in this pull request?
|
Let's open a separate PR for that and get it merged first, so if there are regressions it's isolated to that one PR and it's easy to revert |
@wildan-m can you please fix the workflows |
…x/51296-chat-list-item
…x/51296-chat-list-item
…x/51296-chat-list-item
…x/51296-chat-list-item
PR returning the additional data is in review |
…x/51296-chat-list-item
…x/51296-chat-list-item
…x/51296-chat-list-item
PR returning this data is deployed. Let's retest this PR @allgandalf |
…x/51296-chat-list-item
@luacmartins Great! thanks! |
@luacmartins can you trigger ad-hoc? until you're online i'll post on C+ channel to see if anyone can trigger it before you get back tomorrow |
🚧 @luacmartins has triggered a test build. You can view the workflow run here. |
HEROOO 🙇 |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
@wildan-m @luacmartins Please let me know when this PR is ready for performance testing/profiling |
also please fix conflicts on this one @wildan-m |
…x/51296-chat-list-item
…x/51296-chat-list-item
Co-authored-by: Fábio Henriques <[email protected]>
Co-authored-by: Fábio Henriques <[email protected]>
…wildan-m/App into wildan/fix/51296-chat-list-item
@allgandalf done |
Co-authored-by: Fábio Henriques <[email protected]>
@fabioh8010 do we need to merge this PR for performance testing? if not then i guess this PR is in a very good place for that |
Nice, so I will proceed with performance testing with this PR tomorrow. Please don't merge it yet. |
Explanation of Change
Second PR to add support for all reportAction types in ChatListItem, this part replace chatlistItem inner component with ReportActionItem
Fixed Issues
$ #51296
PROPOSAL: #51296 (comment)
Tests
Pre-condition: Login to account with various chat types
Offline tests
Same as test
QA Steps
Same as test
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Kapture.2024-12-28.at.11.16.37.mp4
Android: mWeb Chrome
Kapture.2024-12-28.at.11.20.10.mp4
iOS: Native
Kapture.2024-12-28.at.09.19.43.mp4
iOS: mWeb Safari
Kapture.2024-12-28.at.09.40.09.mp4
MacOS: Chrome / Safari
Kapture.2024-12-28.at.09.11.12.mp4
MacOS: Desktop
Kapture.2024-12-28.at.11.11.31.mp4